Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process GC preserve #156

Merged
merged 10 commits into from
Jun 25, 2024
Merged

Conversation

udesou
Copy link
Contributor

@udesou udesou commented Jun 11, 2024

Processing GC preserve regions.
Note that objects in these regions are currently put into a separate stack (tpin_gcstack), but later on, when we lower the code for pushing and popping objects into LLVM, we might be able to reuse the gcstack and create a separate frame that we can push and pop. We cannot currently do that because we are currently pushing and popping outside of the current function, using the hook functions.

@udesou udesou requested a review from qinsoon June 11, 2024 10:05
) {
// process transitively pinning stack first
let pinning_closure = if let Some(c) = pclosure {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest having a separate method mmtk_scan_tpin_gcstack or something, and deal with the new stack there. The current code is too confusing: the tpin_gcstack is scanned with the normal closure when pclosure has a value. Creating a new function to scan tpin_gcstack and only pass the closure that is needed to the new function.

Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@udesou udesou merged commit e6def68 into mmtk:v1.9.2+RAI Jun 25, 2024
17 checks passed
qinsoon added a commit that referenced this pull request Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants